Shared CFG: add defaulted getWhileElse/getForeachElse to AstSig by yoff · Pull Request #21931 · github/codeql

yoff · 2026-06-02T13:48:36Z

Lifts the shared-controlflow changes out of #21921 so they can be reviewed independently. No behavioural change for existing languages (Java, C#, Ruby, Swift, Go, …) — both new predicates default to none() and the Make0 loop-edge case extensions only fire when a language overrides them.

Motivation

Python's upcoming new CFG library (#21921) needs to model while-else / for-else, where the else block runs when the loop condition becomes false (rather than via a break). The shared CFG didn't previously have predicates exposing those else blocks.

Changes

Adds defaulted getWhileElse(WhileStmt) and getForeachElse(ForeachStmt) to AstSig (shared/controlflow/codeql/controlflow/ControlFlowGraph.qll).
Extends Make0 in three places (while exit, foreach empty-collection exit, foreach post-body exit) to route through the else block if one exists.

Verification

java/ql/test/library-tests/controlflow/ — all 12 tests pass (CFG output unchanged).
The downstream Python PR (Python: add new shared-CFG-backed control flow graph (additive) #21921) continues to depend on this content; it carries the same hunks inline today and will drop the duplicate when this PR merges and Python: add new shared-CFG-backed control flow graph (additive) #21921 rebases.

Copilot

Pull request overview

Adds shared-CFG signature hooks for loop else blocks so languages that support while-else / for-else (notably Python) can expose that structure to the shared control-flow graph without impacting existing languages.

Changes:

Added defaulted AstSig.getWhileElse(WhileStmt) and AstSig.getForeachElse(ForeachStmt) predicates (default none()).
Updated shared CFG construction (Make0) to route certain loop-exit edges through an else block when provided by a language implementation.
Added a shared/controlflow change note documenting the new signature predicates.

Show a summary per file

File	Description
shared/controlflow/codeql/controlflow/ControlFlowGraph.qll	Extends the shared CFG AST signature with defaulted loop-else accessors and threads them into loop edge construction.
shared/controlflow/change-notes/2026-05-19-loop-else.md	Documents the new defaulted `AstSig` predicates for loop `else` blocks.

Copilot's findings

Files reviewed: 2/2 changed files
Comments generated: 1

+---
+category: feature
+---
+* The `AstSig` signature gains two new defaulted predicates `getWhileElse` and `getForeachElse`, allowing languages (like Python) to model `while-else` / `for-else` constructs where the `else` branch is taken when the loop condition becomes false (rather than via a `break`). Existing languages that do not provide these predicates retain the previous behaviour.


owen-mc · 2026-06-02T15:05:09Z

I wouldn't add a change note. This isn't a user-facing change.

… AstSig Adds three new defaulted signature predicates to the shared CFG library: - getWhileElse / getForeachElse: `else` block of a while/for loop, if any (used by Python's `while-else` / `for-else` constructs). - getCatchType: type expression of a catch clause, if any (used by Python's `except SomeExpr:` where the catch type is a runtime expression that needs CFG evaluation). Each predicate defaults to `none()`, so behaviour is unchanged for any language that doesn't override it (verified by re-running java/ql/test/library-tests/controlflow/). The Make0 succession rules are extended: - WhileStmt/ForeachStmt: route the loop-exit edge through the else block before reaching the after-position. - CatchClause: route the matching-evaluation through the type expression (if present) before reaching the after-value position. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

hvitved · 2026-06-08T08:13:20Z

  default AstNode getTryElse(TryStmt try) { none() }

+  /**
+   * Gets the `else` block of this `while` loop statement, if any.


Suggested change

* Gets the `else` block of this `while` loop statement, if any.

* Gets the `else` block of `while` loop statement `loop`, if any.

hvitved · 2026-06-08T08:13:36Z

+  default AstNode getWhileElse(WhileStmt loop) { none() }
+
+  /**
+   * Gets the `else` block of this `foreach` loop statement, if any.


Suggested change

* Gets the `else` block of this `foreach` loop statement, if any.

* Gets the `else` block of `foreach` loop statement `loop`, if any.

hvitved · 2026-06-08T08:13:51Z

+  default AstNode getForeachElse(ForeachStmt loop) { none() }
+
+  /**
+   * Gets the type expression of this catch clause, if any.


Suggested change

* Gets the type expression of this catch clause, if any.

* Gets the type expression of catch clause `catch`, if any.

hvitved · 2026-06-08T08:19:03Z

          exists(MatchingSuccessor t |
            n1.isBefore(catchclause) and
+            (
+              n2.isBefore(getCatchType(catchclause))


t is unbound in this disjunct.

aschackmull · 2026-06-08T11:18:22Z

+   *
+   * Only some languages (e.g. Python) support `while-else` constructs.
+   */
+  default AstNode getWhileElse(WhileStmt loop) { none() }


Let's stick to just one new predicate

Suggested change

default AstNode getWhileElse(WhileStmt loop) { none() }

default AstNode getLoopElse(LoopStmt loop) { none() }

The semantics of a loop-else block is mainly related to whether the loop exits via its condition or a break, so it's not really tied to the specifics of while vs. foreach.

aschackmull · 2026-06-08T11:18:58Z

+   * statically resolved, this defaults to `none()` and no CFG node
+   * is created.
+   */
+  default Expr getCatchType(CatchClause catch) { none() }


This is unrelated to the PR description - was this meant to go in a different PR?

Also, unless absolutely necessary, I don't think this should be a point of parameterisation - rather, I think we ought to be able to come up with a CFG for catch that solves the needs of all languages. We should probably discuss this offline and put a fix in a different PR when we agree on the solution.

Copilot AI review requested due to automatic review settings June 2, 2026 13:48

yoff requested a review from a team as a code owner June 2, 2026 13:48

Copilot started reviewing on behalf of yoff June 2, 2026 13:48 View session

github-actions Bot added the documentation label Jun 2, 2026

Copilot AI reviewed Jun 2, 2026

View reviewed changes

owen-mc reviewed Jun 2, 2026

View reviewed changes

yoff mentioned this pull request Jun 4, 2026

Python: model exception edges for raise-prone expressions inside try/with #21937

Draft

yoff force-pushed the yoff/python-shared-cfg-loop-else branch from d558371 to 2eaf01c Compare June 5, 2026 07:24

github-actions Bot removed the documentation label Jun 5, 2026

yoff force-pushed the yoff/python-shared-cfg-loop-else branch from 2eaf01c to 4d2296d Compare June 5, 2026 08:12

hvitved requested changes Jun 8, 2026

View reviewed changes

aschackmull reviewed Jun 8, 2026

View reviewed changes

aschackmull requested changes Jun 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shared CFG: add defaulted getWhileElse/getForeachElse to AstSig#21931

Shared CFG: add defaulted getWhileElse/getForeachElse to AstSig#21931
yoff wants to merge 1 commit into
mainfrom
yoff/python-shared-cfg-loop-else

yoff commented Jun 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

owen-mc Jun 2, 2026

Uh oh!

hvitved Jun 8, 2026

Uh oh!

hvitved Jun 8, 2026

Uh oh!

hvitved Jun 8, 2026

Uh oh!

hvitved Jun 8, 2026

Uh oh!

aschackmull Jun 8, 2026

Uh oh!

aschackmull Jun 8, 2026

Uh oh!

aschackmull Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	* Gets the `else` block of this `while` loop statement, if any.
	* Gets the `else` block of `while` loop statement `loop`, if any.

	* Gets the `else` block of this `foreach` loop statement, if any.
	* Gets the `else` block of `foreach` loop statement `loop`, if any.

	* Gets the type expression of this catch clause, if any.
	* Gets the type expression of catch clause `catch`, if any.

	default AstNode getWhileElse(WhileStmt loop) { none() }
	default AstNode getLoopElse(LoopStmt loop) { none() }

Conversation

yoff commented Jun 2, 2026

Motivation

Changes

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants